Files that changed from the base of the PR and between 1a32f05 and 8ef6a05.

• changelog.md (1 hunks)
• e2e/e2etests/test_precompiles_bank.go (8 hunks)
• e2e/e2etests/test_precompiles_bank_through_contract.go (8 hunks)
• precompiles/bank/method_deposit.go (3 hunks)
• precompiles/bank/method_test.go (12 hunks)
• precompiles/bank/method_withdraw.go (4 hunks)
• x/fungible/keeper/zrc20_check_allowance.go (1 hunks)
• x/fungible/keeper/zrc20_check_balance.go (1 hunks)
• x/fungible/keeper/zrc20_check_token_validity.go (1 hunks)
• x/fungible/keeper/zrc20_lock_token.go (1 hunks)
• x/fungible/keeper/zrc20_unlock_token.go (1 hunks)
• x/fungible/types/keys.go (1 hunks)

x/fungible/keeper/zrc20_check_token_validity.go (1)

10-26: 🛠️ Refactor suggestion

Enhance error handling and improve documentation

The IsValidZRC20 function is well-structured and performs the necessary checks. However, there are a few suggestions for improvement:

1. Consider using custom error types instead of fmt.Errorf for better error handling and consistency across the codebase. This approach allows for more granular error checking by callers.

2. Enhance the function documentation by including a code example demonstrating its usage. This will improve the overall package documentation and assist developers in understanding how to use the function correctly.

Here's an example of how you might implement these suggestions:

// ErrZRC20NotWhitelisted is returned when a ZRC20 token is not whitelisted
var ErrZRC20NotWhitelisted = errors.New("ZRC20 is not whitelisted")

// ErrZRC20Paused is returned when a ZRC20 token is paused
var ErrZRC20Paused = errors.New("ZRC20 is paused")

// IsValidZRC20 returns an error whenever a ZRC20 is not whitelisted or paused.
//
// Example usage:
//
//	zrc20Address := common.HexToAddress("0x1234...")
//	err := keeper.IsValidZRC20(ctx, zrc20Address)
//	if err != nil {
//		// Handle the error
//	}
func (k Keeper) IsValidZRC20(ctx sdk.Context, zrc20Address common.Address) error {
	if zrc20Address == zeroAddress {
		return fmt.Errorf("zrc20 address cannot be zero")
	}

	t, found := k.GetForeignCoins(ctx, zrc20Address.String())
	if !found {
		return fmt.Errorf("%w: %s", ErrZRC20NotWhitelisted, zrc20Address.String())
	}

	if t.Paused {
		return fmt.Errorf("%w: %s", ErrZRC20Paused, zrc20Address.String())
	}

	return nil
}

Additionally, it's crucial to ensure comprehensive test coverage for this function. Please add unit tests that cover all branches, including the zero address case, non-whitelisted case, paused case, and the success case. Here's a script to verify the current test coverage:

This script will help identify existing test cases and provide information about the current test coverage for the IsValidZRC20 function.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 11-13: x/fungible/keeper/zrc20_check_token_validity.go#L11-L13
Added lines #L11 - L13 were not covered by tests

---

[warning] 16-18: x/fungible/keeper/zrc20_check_token_validity.go#L16-L18
Added lines #L16 - L18 were not covered by tests

---

[warning] 21-22: x/fungible/keeper/zrc20_check_token_validity.go#L21-L22
Added lines #L21 - L22 were not covered by tests

---

[warning] 25-25: x/fungible/keeper/zrc20_check_token_validity.go#L25
Added line #L25 was not covered by tests

x/fungible/types/keys.go (2)

30-30: Formatting adjustment approved.

The reformatting of the ModuleAddress declaration aligns with Go's standard formatting practices, enhancing code consistency and readability.

---

Line range hint 1-35: Approval for removal of unused init() function.

The removal of the init() function, which contained only commented-out code, is commendable. This change improves code cleanliness and removes potential confusion for future developers.

e2e/e2etests/test_precompiles_bank_through_contract.go (3)

15-15: Import statement addition is appropriate.

The addition of the fungibletypes import is necessary and follows Go conventions for import organization.

---

72-72: Balance check modification is consistent.

The update to use fungibletypes.ModuleAddressZEVM is consistent with the previous change and maintains the logical flow of the test.

---

75-75: Allowance approval modification is appropriate.

The update to use fungibletypes.ModuleAddressZEVM for allowance approval is correct and consistent with the new approach for the bank precompile.

e2e/e2etests/test_precompiles_bank.go (3)

13-13: Import statement addition is appropriate.

The addition of the fungibletypes import is necessary and follows Go conventions.

---

63-63: Approval address update for 500 ERC20ZRC20 tokens is correct.

The change to use fungibletypes.ModuleAddressZEVM is appropriate and maintains consistency with the new addressing scheme.

---

76-76: Approval address update for 1000 ERC20ZRC20 tokens is correct.

The change to use fungibletypes.ModuleAddressZEVM is appropriate and maintains consistency with the new addressing scheme for the bank module.

precompiles/bank/method_test.go (6)

58-59: Consistent update of caller address.

This change is consistent with the previous modification of the caller address.

---

85-86: Consistent update of caller address.

This change is consistent with the previous modifications of the caller address.

---

Line range hint 119-133: Approve address update and deposit amount change.

The caller address update is consistent with previous changes. However, the deposit amount has been increased from 100 to 1000. Please ensure that this modification doesn't adversely affect the test case behavior or expected outcomes.

To verify the impact of the deposit amount change, please review the following:

1. Check if there are any assertions or conditions in the test that depend on the specific deposit amount.
2. Ensure that the increased amount doesn't overflow any limits or cause unexpected behavior in the contract methods being tested.

---

176-177: Consistent updates of caller address.

These changes are consistent with the previous modifications of the caller address from ModuleAddressEVM to ModuleAddressZEVM.

Also applies to: 197-197, 224-225, 278-279, 324-325, 403-404

---

572-575: Approve the update in the allowBank function.

The change from ModuleAddressEVM to ModuleAddressZEVM in the allowBank function is consistent with the previous modifications. This update ensures that the ZEVM module address is approved to spend tokens, aligning with the other changes in the file.

To ensure this change doesn't have unintended consequences, please run the following script:

#!/bin/bash
# Check for any remaining instances of ModuleAddressEVM in relation to token approval
rg --type go "ModuleAddressEVM.*approve"

# Check for the usage of ModuleAddressZEVM in relation to token approval
rg --type go "ModuleAddressZEVM.*approve"

This will help verify that all relevant token approval operations have been updated consistently.

---

31-32: Approve the caller address update.

The change from ModuleAddressEVM to ModuleAddressZEVM is consistent. However, it's crucial to ensure this modification is applied consistently across the entire codebase.

To verify the consistency of this change, please run the following script:

changelog.md (15)

20-21: New feature added: Fungible keeper ability to lock/unlock ZRC20 tokens

This new feature enhances the functionality of the fungible keeper. It's a significant addition that should be thoroughly tested to ensure it doesn't introduce any vulnerabilities or unexpected behavior in token management.

---

Line range hint 54-57: CI pipeline improvements

The fixes to the release pipelines and cleanup steps are important for maintaining a smooth and reliable release process. It's crucial to ensure that these changes have been thoroughly tested in a staging environment before being applied to the production release pipeline.

---

Line range hint 59-62: Chores and documentation updates

The updates to release instructions and the addition of an upgrade handler for version v12.1.0 are important for maintaining clear documentation and smooth upgrade processes. Ensure that the upgrade handler has been thoroughly tested with various upgrade scenarios.

---

Line range hint 187-193: Chores and minor updates

The chores and minor updates, such as renaming files and updating build configurations, contribute to better code organization and build processes. Ensure that these changes don't introduce any unexpected behavior in the build or runtime environments.

---

Line range hint 198-199: CI improvements

The removal of private runners and unused GitHub Action contributes to a cleaner and more maintainable CI setup. Ensure that all necessary CI processes are still in place and functioning correctly after these removals.

---

Line range hint 1-199: Overall assessment of the changelog

The changelog demonstrates significant development efforts across multiple versions, with a focus on enhancing functionality, reliability, and performance. Key points to note:

1. Version v12.0.0 introduces several breaking changes, particularly in module organization and query endpoints. These changes will require careful migration planning for users.

2. New features have been added consistently, including monitoring capabilities, dynamic gas pricing, and improved transaction handling.

3. Numerous fixes address critical issues in various areas such as transaction processing, chain validation, and event handling.

4. Refactoring efforts have improved code organization and maintainability.

5. Test coverage has been expanded, although there's room for more comprehensive testing.

6. CI/CD processes have been streamlined and improved.

Action items:

1. Develop and publish a comprehensive migration guide for the breaking changes in v12.0.0.
2. Conduct thorough testing of new features and fixes, especially under high load conditions and edge cases.
3. Continue expanding test coverage, particularly for complex scenarios involving multiple modules.
4. Monitor the effects of dynamic gas pricing in production environments.
5. Update all documentation to reflect the latest changes, especially regarding new and modified endpoints.

Overall, the changelog reflects a project in active development with a strong focus on improvement and stability. The breaking changes, while potentially disruptive, seem to be in service of a more organized and maintainable codebase.

---

Line range hint 39-52: Multiple fixes and improvements in version v12.1.0

This version includes several important fixes and improvements:

1. Avoiding voting on wrong ballots due to false blockNumber in EVM tx receipt
2. Fixing chain params comparison logic
3. Exempting system transactions from min gas price check and gas fee deduction
4. Setting keygen status correctly
5. Fixing zetaclient crash issues
6. Skipping unsupported chain parameters

These changes address critical issues and improve the robustness of the system. However, it's important to ensure that these fixes don't introduce new edge cases or unexpected behavior.

To verify the impact of these changes, especially for EVM-related fixes, we can run the following script:

#!/bin/bash
# Description: Check for any remaining references to old EVM transaction handling

# Test: Search for old EVM transaction handling methods
rg --type go 'EVM.*Transaction' --glob '!changelog.md'

---

Line range hint 64-68: New features and support added

The addition of support for lower gas limits, improvements to inbound transaction observation, and the integration of authenticated calls are significant enhancements. These features should be thoroughly tested, especially in edge cases and under high load conditions.

To verify the implementation of these features, we can run the following script:

#!/bin/bash
# Description: Check for the implementation of new features

# Test: Search for lower gas limit implementation
rg 'lower.*gas.*limit' --type go

# Test: Search for inbound transaction observation improvements
rg 'inbound.*transaction.*observation' --type go

# Test: Search for authenticated calls integration
rg 'authenticated.*calls' --type go

---

Line range hint 70-74: Code refactoring and optimizations

The refactoring efforts, including the reorganization of zetaclient into subpackages and the addition of EVM fee calculation to TSS migration, are positive steps towards improving code organization and maintainability. However, ensure that these changes don't introduce any regressions or unexpected behavior.

To verify the impact of the refactoring, we can run the following script:

#!/bin/bash
# Description: Check for any remaining references to old package structure

# Test: Search for old package references
rg 'zetaclient/[^/]+/' --type go --glob '!zetaclient/'

---

Line range hint 154-169: Multiple fixes in v12.0.0

The fixes address various issues including UTXO handling, EVM chain outbound transaction processing, and event sanity checks. These improvements enhance the reliability and security of the system. However, ensure that these fixes have been thoroughly tested, especially in edge cases and under high load conditions.

To verify the impact of these fixes, we can run the following script:

#!/bin/bash
# Description: Check for any remaining references to old transaction handling methods

# Test: Search for old UTXO handling methods
rg 'UTXO' --type go --glob '!changelog.md'

# Test: Search for old EVM outbound transaction handling
rg 'EVM.*outbound.*transaction' --type go --glob '!changelog.md'

# Test: Search for event sanity checks
rg 'event.*sanity.*check' --type go

---

Line range hint 145-152: New features added in v12.0.0

The addition of monitoring capabilities, state variables for aborted zeta amounts, and new commands are positive enhancements. The enablement of dynamic gas prices on zetachain is a significant change that should be carefully monitored in production for any unexpected effects on transaction processing or user costs.

To verify the implementation of these features, we can run the following script:

#!/bin/bash
# Description: Check for the implementation of new features in v12.0.0

# Test: Search for monitoring implementation
rg 'grafana|prometheus|ethbalance' --type go

# Test: Search for aborted zeta amount tracking
rg 'aborted.*zeta.*amount' --type go

# Test: Search for dynamic gas price implementation
rg 'dynamic.*gas.*price' --type go

---

Line range hint 171-185: Refactoring and improvements in v12.0.0

The refactoring efforts, including the consolidation of node builds and updates to contract bytecode management, contribute to improved code organization and maintainability. The changes to queries for large data sets by adding pagination are particularly important for scalability.

To verify the impact of the refactoring and improvements, we can run the following script:

#!/bin/bash
# Description: Check for implementation of pagination and new query structures

# Test: Search for pagination implementation
rg 'pagination' --type go

# Test: Search for new query structures
rg 'grpc.*query|msg.*query' --type go

---

Line range hint 23-30: Multiple fixes in version v12.2.4

The fixes address several important issues:

1. Ensuring external chain height always increases
2. Buffering EIP1559 gas price increase
3. Changing authorization for WhitelistERC20
4. Handling pending outbound transaction hashes
5. Scheduling bitcoin keysign with intervals
6. Skipping Goerli BlobTxType transactions
7. Handling Mumbai empty blocks
8. Using estimated SegWit tx size for Bitcoin gas fee calculation
9. Using chain param ConfirmationCount for bitcoin confirmation

These fixes appear to address critical issues and improve the overall stability and functionality of the system. However, it's important to ensure that these changes don't introduce new issues or regressions.

To verify the impact of these changes, especially for Bitcoin-related fixes, we can run the following script:

---

Line range hint 35-37: Modification to emission distribution

The change to use fixed block rewards for emission distribution is a significant alteration to the economic model of the system. This change should be carefully reviewed to ensure it aligns with the project's tokenomics goals and doesn't introduce any unintended consequences.

To verify the implementation of this change, we can run the following script:

---

Line range hint 32-33: Removal of standalone network and update to testing package

This change simplifies the testing setup and potentially improves test consistency. However, it's crucial to ensure that all existing tests have been updated to work with the new testing package and that test coverage hasn't been reduced.

To verify the impact of this change, we can run the following script:

✅ Verification successful

Removal of Standalone Network References Verified

All references to the standalone network have been successfully removed or updated. The testing setup remains simplified and consistent with the new testing package.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to standalone network

# Test: Search for standalone network references
rg 'standalone.*network' --glob '!changelog.md'

Length of output: 6062

---

Script:

#!/bin/bash
# Description: Thoroughly check for any remaining references to "standalone network" in the codebase

# Search for exact phrase "standalone network" with word boundaries
rg '\\bstandalone\\s+network\\b' --glob '!changelog.md'

# Search for variations like "standaloneNetwork" or "StandaloneNetwork"
rg '\\bstandaloneNetwork\\b' --glob '!changelog.md'
rg '\\bStandaloneNetwork\\b' --glob '!changelog.md'

# Search for imports or related terms that might reference standalone network components
rg 'import.*standalone' --glob '!changelog.md'
rg 'standalone.*import' --glob '!changelog.md'

Length of output: 263

x/fungible/keeper/zrc20_check_allowance.go (1)

36-48: Ensure correct EVM call parameters

Verify that the parameters passed to CallEVM are correct for invoking the allowance method. Specifically, ensure that the caller address fungibletypes.ModuleAddressZEVM is appropriate and that the flags for gas estimation and state transition (true, true) align with the intended read-only operation.

Would you like to confirm that CallEVM is correctly configured for this context?

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 36-50: x/fungible/keeper/zrc20_check_allowance.go#L36-L50
Added lines #L36 - L50 were not covered by tests

x/fungible/keeper/zrc20_unlock_token.go (1)

13-14: ⚠️ Potential issue

Define 'zeroAddress' to avoid undefined identifier

The variable zeroAddress is used in the address checks but is not defined within this context. This will result in a compilation error due to an undefined identifier.

Define zeroAddress as follows:

 package keeper

+import "github.com/ethereum/go-ethereum/common"
+
+var zeroAddress = common.Address{}
 
 const transfer = "transfer"

Alternatively, you can replace zeroAddress with common.Address{} directly in the checks.

Likely invalid or redundant comment.

precompiles/bank/method_deposit.go (1)

21-21: Clarification of Required Permissions

The updated comment correctly specifies that the caller must allow the Fungible ZEVM address to spend ZRC20 tokens on their behalf. This enhances clarity regarding the necessary permissions for the deposit process.